Implement ITextProvider and ITextRangeProvider for UIA - #38538
Conversation
| auto current_line_start = start->Clone(); | ||
| while (!current_line_start->IsNullPosition() && *current_line_start < *end) { | ||
| auto current_line_end = current_line_start->CreateNextLineEndPosition( | ||
| AXBoundaryBehavior::CrossBoundary); |
There was a problem hiding this comment.
Follow-up to this conversation: #38284 (comment)
@loic-sharma: Could you explain why AXBoundaryDetection::kDontCheckInitialPosition was removed here?
@yaakovschectman:
The method signature for our version of CreateNextLineEndPosition takes a parameter of a different type than Chrome's current version of the same. We take a AXBoundaryBehavior, while Chromium takes a struct consisting of AXBoundaryBehavior, AXBoundaryDetection
Why are our method signatures different? Should we update our signatures to match Chromium's? Will we get behavioral differences if we don't update (either here or in other places that use CreateNextLineEndPosition with a non-default AXBoundaryDetection)?
There was a problem hiding this comment.
This is not only a superficial signature difference, our implementation is different also. We do not have the AXMovementOptions struct that exists in Chromium. Updating it in our existing code may warrant a separate PR.
There was a problem hiding this comment.
Right. What are the resulting behavior differences while we don't have AXMovementOptions?
There was a problem hiding this comment.
Presumably, there will not be an inherent difference between checking or not checking the initial position when moving by a boundary. I think for our purposes a similar effect can be achieved by the conditions used where the enclosing methods are called.
| size_t* find_length, | ||
| bool ignore_case, | ||
| bool backwards) { | ||
| // TODO(schectman) Respect ignore_case |
There was a problem hiding this comment.
Do we also need to add internationalization support?
There was a problem hiding this comment.
Maybe. I do not know what actual use case calls this method.
There was a problem hiding this comment.
What do you think of updating the issue/comment to mention both internalization and case insensitivity?
| case TextUnit_Character: | ||
| new_position = | ||
| MoveEndpointByCharacter(position_to_move, count, units_moved); | ||
| break; |
There was a problem hiding this comment.
Why was TextUnit_Format removed? Should there be a TODO?
There was a problem hiding this comment.
I don't know, I will see if it can be added
There was a problem hiding this comment.
I see. We do not actually have kFormatStart and kFormatEnd in our TextBoundary enum.
There was a problem hiding this comment.
Should we create an issue and leave a TODO then? This seems like a useful thing: https://github.com/MicrosoftDocs/win32/blob/docs/desktop-src/WinAuto/uiauto-uiautomationtextunits.md#format
|
The code changes seem good, nice work! Could you detail your test plan once text providers are completed? Will it be manual validation using a screen reader? If so, what scenarios will you test? |
e44c800 to
681f359
Compare
8f6aa39 to
6ac9db4
Compare
|
@loic-sharma In particular, hovering the cursor over an element, moving the focus using the keyboard, and triggering a read of the current element (e.g. NVDA+Up). Once TextProviders are fully implemented, this will also include moving the carat, erasing characters, and changing the selection using the keyboard. |
loic-sharma
left a comment
loic-sharma
left a comment
There was a problem hiding this comment.
LGTM modulo pending comments. Nice work!
2 participants
This is the initial subset of #38284, handling importing files from Chromium, making modifications necessary to compile, and temporarily disabling the newly added unittests.
In order for text providing nodes to properly interact with screen reader commands via UIA, we need to implement text and textedit pattern providers. As an example, this allows screen readers to narrate the character after the carat when the user moves the carat in an edit field with the arrow keys.
Unit tests for
AXPlatformNodeText(Range)ProviderWinare modeled after those found in the Chromium source. Details for some required modification to account for differences between the two codebases, and a handful of the newly introduced test cases are currently disabled, which is tracked in flutter/flutter#117012.Functionality for searching for text in a text range does not yet respect the
ignore_caseoption: flutter/flutter#117013.Part of flutter/flutter#116219
Pre-launch Checklist
///).